fix(@schematics/angular): don't show server routing prompt when using browser builder
#28878
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dgp1130
added
action: review
dgp1130
force-pushed
the
ssr-routing-prompt
branch
3 times, most recently
from
b9890e9 to
9d3097f
Compare
jkrems
approved these changes
Nov 18, 2024
|
Looks like some test(s) hang with interactive prompts right now? |
clydin
approved these changes
Nov 18, 2024
… `browser` builder The new routing APIs don't support `browser` builder, but calling `ng add @angular/ssr` with a `browser` builder would still prompt the user to add them. If the user said "Yes", it would actually ignore that answer and not enable the new APIs. With this change, `ng add @angular/ssr` when using `browser` builder does not show the prompt and assumes the answer is "No". It also throws an error if the user runs `ng add @angular/ssr --server-routing`. I'm not aware of a built-in prompting mechanism in schematics beyond `x-prompt`, which can't be used here, so instead I just called Inquirer directly. Unfortunately testing the prompt is a little awkward, as Inquirier does not provide useful APIs in this space. I evaluated `@inquirer/testing`, but ultimately decided that was more intended for testing custom Inquirer prompts, not mocking usage of standard prompts. Schematics APIs do not provide a useful way to inject additional data like a mock, so instead I had to do this through a `setPrompterForTestOnly` function. I'm not a huge fan of it, but I don't see a more straightforward way of solving the problem.
dgp1130
force-pushed
the
ssr-routing-prompt
branch
from
9d3097f to
0db7534
Compare
dgp1130
added
action: merge
|
The changes were merged into the following branches: main, 19.0.x |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The new routing APIs don't support
browserbuilder, but callingng add @angular/ssrwith abrowserbuilder would still prompt the user to add them. If the user said "Yes", it would actually ignore that answer and not enable the new APIs.With this change,
ng add @angular/ssrwhen usingbrowserbuilder does not show the prompt and assumes the answer is "No". It also throws an error if the user runsng add @angular/ssr --server-routing.I'm not aware of a built-in prompting mechanism in schematics beyond
x-prompt, which can't be used here, so instead I just called Inquirer directly. Unfortunately testing the prompt is a little awkward, as Inquirier does not provide useful APIs in this space. I evaluated@inquirer/testing, but ultimately decided that was more intended for testing custom Inquirer prompts, not mocking usage of standard prompts. Schematics APIs do not provide a useful way to inject additional data like a mock, so instead I had to do this through asetPrompterForTestOnlyfunction. I'm not a huge fan of it, but I don't see a more straightforward way of solving the problem.